Skip to content

Unify timestamp checks#57

Merged
elichad merged 8 commits intodevelopfrom
unify-timestamp-checks
Jan 5, 2026
Merged

Unify timestamp checks#57
elichad merged 8 commits intodevelopfrom
unify-timestamp-checks

Conversation

@elichad
Copy link
Copy Markdown

@elichad elichad commented Jan 5, 2026

Alternative to #56 focused only on unifying the approach to timestamp validation. Adds a combined check into the new file 3_timestamp_format.ttl as I noticed we haven't used the index 3 for any other checks. Example error message:

     [ five-safes-crate-0.4_23 ]: Timestamp Format                                                                                    
                                                                                                                                      
      Timestamps MUST follow the RFC 3339 standard (YYYY-MM-DD'T'hh:mm:ss[.fraction](Z | ±hh:mm)).                                    
                                                                                                                                      
          Failed checks                                                                                                               
                                                                                                                                      
       [ five-safes-crate-0.4_23.0 ] Timestamp Format:                                                                                
                                      Timestamps MUST follow the RFC 3339 standard (YYYY-MM-DD'T'hh:mm:ss[.fraction](Z | ±hh:mm)).    
         Detected issues                                                                                                              
         - [Violation"01 Jan 2026 10:00"  on <01 Jan 2026 10:00>]: All `startTime` and `endTime` values MUST follow the RFC           
         3339 standard (YYYY-MM-DD'T'hh:mm:ss[.fraction](Z | ±hh:mm)).

Downside: the error messages don't state which entity the problem timestamp is from. But I think overall it's cleaner than the option presented in #56/#50

@elichad elichad mentioned this pull request Jan 5, 2026
Closed
@elichad
Copy link
Copy Markdown
Author

elichad commented Jan 5, 2026

This approach could also be incorporated into the generic "Review Process" check set that we were considering making (i.e. checks that are applicable to all the different phases)

douglowe
douglowe requested changes Jan 5, 2026
View reviewed changes
Copy link
Copy Markdown

@douglowe douglowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an elegant solution. There's a couple places we have redundant code now, which could be removed.

Comment thread rocrate_validator/profiles/five-safes-crate/must/12_check_phase.ttl Outdated
Comment thread rocrate_validator/profiles/five-safes-crate/must/12_check_phase.ttl Outdated
Comment thread rocrate_validator/profiles/five-safes-crate/must/13_validation_phase.ttl Outdated
Comment thread rocrate_validator/profiles/five-safes-crate/must/13_validation_phase.ttl Outdated
Comment thread rocrate_validator/profiles/five-safes-crate/must/8_disclosure_phase.ttl Outdated
Comment thread rocrate_validator/profiles/five-safes-crate/must/8_disclosure_phase.ttl Outdated
Comment thread rocrate_validator/profiles/five-safes-crate/should/4_sign_off.ttl Outdated
douglowe
douglowe approved these changes Jan 5, 2026
View reviewed changes
Copy link
Copy Markdown

@douglowe douglowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me now.

@elichad elichad merged commit 6299e47 into develop Jan 5, 2026
4 checks passed
@douglowe douglowe linked an issue Jan 5, 2026 that may be closed by this pull request
Ensure timestamp checks are consistent #50
Closed
@douglowe douglowe mentioned this pull request Jan 5, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@douglowe douglowe douglowe approved these changes

@alexhambley alexhambley Awaiting requested review from alexhambley

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ensure timestamp checks are consistent

2 participants

@elichad @douglowe